feat(webapp): run the dashboard agent through AWS Bedrock behind an env switch - #4609
feat(webapp): run the dashboard agent through AWS Bedrock behind an env switch#4609kathiekiwi wants to merge 25 commits into
Conversation
…switch DASHBOARD_AGENT_MODEL_PROVIDER=bedrock routes the agent's model calls through AWS Bedrock; the default stays direct Anthropic. Managed prompts keep canonical "anthropic:<model-id>" strings, resolved per provider in one place alongside the provider-shaped prompt-cache options.
Bedrock reports its cache write on the call's provider metadata and its cache read only on the call's usage, so the per-step cache telemetry reads whichever the active provider populates.
…ed fallback Confirm both Bedrock profiles against the SDK model-id union, throw on an unmapped id instead of guessing an unqualified profile, and add a structural shape test.
authenticateUserActor now applies the same membership floor as authenticateSession: a delegated token for a user who is not a member of the scoped org/project is denied, instead of being handed a usable (read:all-by-default) ability. Platform admins stay exempt.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 6 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (38)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (11)📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-06-13T19:53:13.759ZApplied to files:
📚 Learning: 2026-06-17T17:13:49.929ZApplied to files:
📚 Learning: 2026-06-23T13:04:21.413ZApplied to files:
📚 Learning: 2026-05-18T14:40:02.173ZApplied to files:
📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
📚 Learning: 2026-06-09T17:58:04.699ZApplied to files:
📚 Learning: 2026-06-16T09:19:47.637ZApplied to files:
🪛 ast-grep (0.45.1)internal-packages/dashboard-agent/src/model-provider.test.ts[error] 35-38: Recursive/iterative merge copies attacker-controllable keys from a source object into a target via a computed property assignment without rejecting dangerous keys, allowing prototype pollution. Skip or block "proto", "constructor", and "prototype" keys (e.g. (prototype-pollution-recursive-merge-typescript) 🔇 Additional comments (1)
WalkthroughThe dashboard agent now supports Anthropic and Bedrock model providers. Environment validation accepts provider-specific AWS configuration. Shared model resolution and cache-breakpoint helpers replace Anthropic-specific runtime logic. Cache telemetry reads provider metadata and usage fallbacks. Delegated user-actor authentication now validates token subjects and enforces organization and project membership, with integration test coverage. Merge Risk: 🟡 Moderate · up to The PR adds an environment-selected model provider and strengthens scoped tenant authorization, but merge readiness is reduced because the project configuration may skip type-checking the RBAC source and tests, while provider-test cleanup can affect later tests through inherited environment values. These issues should receive explicit owner follow-up before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Only load the user and run the membership floor when organizationId or projectId is present, restoring pre-P5 behavior for unscoped tokens (no user query, no denial). The user lookup falls back to the primary before failing closed, so replica lag can't spuriously 401 a just-created member.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…er switch The sampled-turn eval task resolved its judge with Anthropic directly, so under the Bedrock switch with no Anthropic key every sampled eval failed. It now follows the same provider seam as the agent.
…versations On Bedrock the turn-wide prefix marker and the per-step marker were byte-identical, so the step-strip pass removed the prefix on short chats, leaving the history uncached every step. The step marker now carries an explicit ttl so the prefix survives, matching the Anthropic path.
Use a nonexistent userId for the unscoped case so success can only hold if the lookup is skipped.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…EFAULT_REGION The Bedrock SDK reads only AWS_REGION; pass region explicitly so AWS_DEFAULT_REGION (common in AWS deploys) works too. Credentials stay on the SDK's own chain.
…ing the webapp gate
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The Bedrock step marker discriminated itself with a ttl inside cachePoint, which the SDK copies verbatim into the Converse request. Move the prefix/step discriminator to a non-serialized top-level providerOptions key so the wire cachePoint is a plain {type:"default"}. The key's value is an object because the AI SDK validates message providerOptions as records.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…criminator Conversations persisted before __cacheBreakpoint existed carry a bare anthropic.cacheControl. Classify it by ttl and strip it on resume so legacy breakpoints don't slip past the 4-breakpoint limit.
Split into tsconfig.src.json/tsconfig.test.json like run-engine and schedule-engine, so fallback.userActor.test.ts is type-checked instead of excluded.
Add DASHBOARD_AGENT_AWS_ACCESS_KEY_ID/SECRET_ACCESS_KEY/REGION so the agent's Bedrock creds don't hijack the default AWS chain the ECR/STS deploy clients rely on. Falls back to the default chain and existing AWS_REGION/AWS_DEFAULT_REGION order when unset.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…bedrock # Conflicts: # pnpm-lock.yaml
tsc -b failed: referenced src/test configs use noEmit without composite. Base tsconfig.json is now a plain IDE-default project, unrelated to the split -p configs the typecheck script uses.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
…ider tests afterEach deleted these unconditionally, which could clobber CI-provided AWS config for later test files. Snapshot in beforeEach and restore the exact prior value (or delete if previously unset).
| // Selects the dashboard agent's LLM provider (default anthropic). The internal | ||
| // seam reads process.env directly; this entry validates the value webapp-side. | ||
| DASHBOARD_AGENT_MODEL_PROVIDER: z.enum(["anthropic", "bedrock"]).default("anthropic"), |
There was a problem hiding this comment.
🟡 Leaving the new model-provider setting blank stops the dashboard from starting
The new provider setting is validated with a strict choice list that has no blank handling (z.enum(["anthropic", "bedrock"]).default("anthropic") at apps/webapp/app/env.server.ts:214), so a deployment that defines the variable with an empty value fails validation instead of falling back to the default, and the whole app refuses to boot.
Impact: An operator who templates the variable in but leaves it empty takes the entire webapp down at startup rather than getting the default provider.
Why a blank value is not treated as unset
Zod's .default() only applies when the value is undefined. process.env.DASHBOARD_AGENT_MODEL_PROVIDER = "" is a defined string, so z.enum rejects it and EnvironmentSchema.parse(process.env) (apps/webapp/app/env.server.ts:2438) throws at module load, killing boot.
The rest of this schema explicitly guards against exactly this: OptionalBoolEnv, BoolEnvWithDefault, IntEnvWithDefault and OptionalLimitEnv (apps/webapp/app/env.server.ts:117-147) all normalise blank/whitespace to undefined so it "falls back to the default instead of parsing as false/0". A z.preprocess that maps a blank/whitespace string to undefined before the enum keeps the same convention. Note the internal seam (internal-packages/dashboard-agent/src/model-provider.ts:20-22) already treats any non-"bedrock" value, including "", as anthropic — so the strict webapp-side check is the only thing that turns a blank value into an outage.
| // Selects the dashboard agent's LLM provider (default anthropic). The internal | |
| // seam reads process.env directly; this entry validates the value webapp-side. | |
| DASHBOARD_AGENT_MODEL_PROVIDER: z.enum(["anthropic", "bedrock"]).default("anthropic"), | |
| // Selects the dashboard agent's LLM provider (default anthropic). The internal | |
| // seam reads process.env directly; this entry validates the value webapp-side. | |
| // Blank/whitespace normalises to undefined so it falls back to the default. | |
| DASHBOARD_AGENT_MODEL_PROVIDER: z.preprocess( | |
| (v) => (typeof v === "string" && v.trim() === "" ? undefined : v), | |
| z.enum(["anthropic", "bedrock"]).default("anthropic") | |
| ), |
Was this helpful? React with 👍 or 👎 to provide feedback.
| export const BEDROCK_MODEL_IDS: Record<string, string> = { | ||
| "claude-sonnet-4-6": "us.anthropic.claude-sonnet-4-6-v1", | ||
| "claude-haiku-4-5": "us.anthropic.claude-haiku-4-5-20251001-v1:0", | ||
| }; |
There was a problem hiding this comment.
🔍 Bedrock inference-profile ids are only structurally validated, never resolved
BEDROCK_MODEL_IDS maps claude-sonnet-4-6 to an undated us.anthropic.claude-sonnet-4-6-v1 while claude-haiku-4-5 uses the dated ...-20251001-v1:0 form. The test only asserts a regex shape, which by construction cannot catch a wrong-but-well-formed id; the PR description already flags that the Sonnet profile needs a live smoke test. A wrong id fails only at first invocation with a 404, and on the head-start path that surfaces as the generic "couldn't start this response" turn, so the failure mode is quiet — verify against the live Bedrock model list before flipping the switch.
Was this helpful? React with 👍 or 👎 to provide feedback.
What & why
The dashboard agent can now run its model calls through AWS Bedrock instead of the direct Anthropic API, chosen by a single env switch. It's off by default (
DASHBOARD_AGENT_MODEL_PROVIDERunset ⇒anthropic), so merging changes nothing at runtime — the Bedrock path is a dormant branch until an operator sets the switch and AWS config. The default Anthropic path is byte-for-byte unchanged.This also carries a related tenant-isolation hardening for the agent's delegated token (kept together deliberately — both land the agent on Bedrock for HIPAA readiness). Refs: TRI-13251, TRI-11032.
What's inside
Provider seam —
internal-packages/dashboard-agent/src/model-provider.ts: the registry now holds bothanthropicandbedrock;resolveDashboardAgentModel()maps the canonical"anthropic:<id>"strings the managed prompts carry to the active provider, and the cache-breakpoint helpers emit the active provider's shape — AnthropiccacheControlvs BedrockcachePoint. Managed prompt strings stay canonical, so stored prompts don't change meaning. Unmapped model ids throw rather than shipping a guaranteed-404 profile. All agent, watch, compaction and title callsites route through the resolver; thedashboardAgentModelKeylocals override (test mock injection) is preserved.Cache telemetry —
step-cache.ts: cache token usage is read from the active provider (Anthropic reports it on provider metadata; Bedrock reports the write on metadata and the read via standard usage), sogen_ai.usage.cache_*is populated on both. This also fixes a latent ordering bug where step attributes could null-overwrite the prompt-cache read count.Webapp callsites —
dashboardAgentHeadStart.server.tsand the head-start route resolve the model and the cache breakpoint through the same seam, so the warm-up prefix and the following turn share one provider. The head-start firing gate is provider-aware: on Bedrock it gates onAWS_REGIONand lets the SDK resolve credentials (IAM role / static keys / session token / bearer), so a role-based deploy still warms; on Anthropic it staysBoolean(ANTHROPIC_API_KEY).app/env.server.tsgains the optional AWS vars and validatesDASHBOARD_AGENT_MODEL_PROVIDER.ANTHROPIC_API_KEYis untouched and not required on a Bedrock deploy.Tenant-isolation hardening —
internal-packages/rbac/src/fallback.ts: for a scoped context, the OSSauthenticateUserActornow applies the same membership floor as the session path — a delegated user-actor token whose user is not a member of the scoped org/project is denied (403). Unscoped tokens keep their prior behavior (no tenant claim, no lookup). The user lookup falls back replica→primary so replication lag can't spuriously 401 a just-joined member. Members and admins are unaffected. Previously this invariant held only through per-route discipline; this makes it structural.Enabling Bedrock (later, ops)
DASHBOARD_AGENT_MODEL_PROVIDER=bedrockidentically in both the webapp and the agent task container — the webapp warms the cache prefix and the task reads it, so a split would silently miss the cache.AWS_REGIONand provide credentials the Bedrock SDK can resolve (IAM role preferred). For v1 this runs without an Anthropic API key. Note: with no Anthropic key set, rollback is "turn the agent off", not "unset the switch" (unsetting falls back to the Anthropic provider, which then has no key).Testing
Unit tests cover both provider paths: the provider switch and per-provider cache shapes, a structural regex asserting Bedrock ids are real inference profiles (not an echo of the table), the split-metadata cache telemetry, and real-Postgres RBAC tests — member allowed, scoped non-member denied (org-only and project-only), missing user → 401, admin non-member exempt, unscoped success.
typecheck --filter webappand the dashboard-agent + rbac suites pass.